core&ui: assignment auto complete - #1061

packages/ui-default/components/autocomplete/AssignSelectAutoComplete.tsx (1)

7-17: Remove hardcoded multi: true to respect caller's configuration.

The constructor hardcodes multi: true in props (line 12) but then spreads options (line 15), allowing the hardcoded value to be overridden. This creates ambiguity about the intended behavior.

Since the generic parameter Multi suggests configurability and the caller in contest_edit.page.ts explicitly passes multi: true, the hardcoded value is redundant.

Apply this diff to remove the redundancy:

   constructor($dom, options: AutoCompleteOptions<Multi> = {}) {
     super($dom, {
       classes: 'assign-select',
       component: AssignSelectAutoCompleteFC,
       props: {
-        multi: true,
         height: 'auto',
       },
       ...options,
     });
   }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/hydrooj/src/handler/domain.ts (1)

440-440: Mark domainId as required in the schema.

The domainId parameter should be marked as required to prevent potential runtime errors when args.domainId is undefined.

Apply this diff:

-            domainId: Schema.string(),
+            domainId: Schema.string().required(),

packages/ui-default/components/autocomplete/components/AssignSelectAutoComplete.tsx (1)

104-114: Remove isRequired from onChange PropType.

The onChange prop is marked as isRequired, but the underlying AutoComplete component makes it optional (defaulting to a no-op). The PropType should reflect this optionality.

Based on past review comments.

Apply this diff:

 AssignSelectAutoComplete.propTypes = {
   width: PropTypes.string,
   height: PropTypes.string,
   listStyle: PropTypes.object,
-  onChange: PropTypes.func.isRequired,
+  onChange: PropTypes.func,
   multi: PropTypes.bool,
   selectedKeys: PropTypes.arrayOf(PropTypes.string),
   allowEmptyQuery: PropTypes.bool,
   freeSolo: PropTypes.bool,
   freeSoloConverter: PropTypes.func,
 };

packages/ui-default/components/autocomplete/components/AssignSelectAutoComplete.tsx (2)

73-89: Simplify the renderItem structure.

The conditional rendering of the avatar creates an asymmetric structure. Consider refactoring to always render a media__left container and conditionally render its content, which improves consistency and maintainability.

Based on past review comments.

Apply this diff:

     renderItem={(item) => (
       <div className="media">
-        {item.type === 'user' ? (
-          <div className="media__left medium">
+        <div className="media__left medium">
+          {item.type === 'user' && (
             <img className="small user-profile-avatar" alt="" src={item.avatarUrl} width="30" height="30" />
-          </div>
-        ) : null}
+          )}
+        </div>
         <div className="media__body medium">
           <div className="assign-select__name">
             {item.name}{item.type === 'user' && item.displayName && ` (${item.displayName})`}
           </div>
           <div className="assign-select__desc">
             {item.type === 'group' ? <>Group • {item.uids?.length || 0} users</> : <>User • UID = {item.key}</>}
           </div>
         </div>
       </div>
     )}

---

104-114: Consider removing PropTypes in favor of TypeScript.

The component uses both TypeScript types (AutoCompleteProps<AssignItem>) and runtime PropTypes validation. Since TypeScript provides compile-time type safety, PropTypes add maintenance overhead without significant benefit in a TypeScript codebase. Consider removing PropTypes unless runtime validation is specifically required for this component.

If you decide to keep PropTypes, ensure they stay in sync with the TypeScript types.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/ui-default/components/autocomplete/components/AssignSelectAutoComplete.tsx (2)

51-51: Missing avatarUrl in fetchItems projection.

The fetchItems API call on line 51 requests ['_id', 'uname', 'displayName'] but omits 'avatarUrl', which is used by toUserItem (line 22) and rendered in the UI (line 77). This will cause avatars to be missing when items are fetched by keys.

Apply this diff to include avatarUrl:

       const [users, groups]: [Udoc[], GDoc[]] = await Promise.all([
-        userIds.length > 0 ? api('users', { auto: userIds }, ['_id', 'uname', 'displayName']) : [],
+        userIds.length > 0 ? api('users', { auto: userIds }, ['_id', 'uname', 'displayName', 'avatarUrl']) : [],
         groupNames.length > 0 ? api('groups', { names: groupNames }, ['name', 'uids']) : [],
       ]);

---

108-108: Remove .isRequired from onChange PropType.

The onChange prop is marked as required, but the underlying AutoComplete component makes it optional (defaulting to a no-op function on line 89 of AutoComplete.tsx). The PropType should match the TypeScript interface.

Apply this diff:

-  onChange: PropTypes.func.isRequired,
+  onChange: PropTypes.func,

packages/ui-default/components/autocomplete/components/AssignSelectAutoComplete.tsx (1)

56-61: Consider visual styling for invalid groups.

Lines 56-61 correctly handle missing/deleted groups by marking them with invalid: true, and line 67 displays "(invalid)" text. However, the invalid state could be more prominent in the rendered item (lines 73-89) to better alert users.

Consider adding visual styling for invalid items:

     renderItem={(item) => (
-      <div className="media">
+      <div className={`media${item.invalid ? ' invalid' : ''}`}>
         {item.type === 'user' && (
           <div className="media__left medium">
             <img className="small user-profile-avatar" alt="" src={item.avatarUrl} width="30" height="30" />
           </div>
         )}
         <div className="media__body medium">
           <div className="assign-select__name">
             {item.name}{item.type === 'user' && item.displayName && ` (${item.displayName})`}
+            {item.invalid && <span className="invalid-badge"> (invalid)</span>}
           </div>

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

packages/components/frontend/autocomplete/AutoComplete.tsx (1)

115-124: Consider structured error handling for production.

While the error logging is functional, consider these optional improvements:

1. Structured logging: Replace console.error with a proper logging service for production monitoring
2. Error callback API: Add an onError?: (error: Error, context: string) => void prop to allow parent components to handle errors (show notifications, track metrics, etc.)
3. Error recovery: For the paste operation specifically, consider partial success handling (add items that succeeded, notify about failures)

Example API enhancement:

export interface AutoCompleteProps<Item> {
  // ... existing props
  onError?: (error: Error, context: 'query' | 'fetch' | 'paste') => void;
}

Then use it in error handlers:

 } catch (e) {
   console.error('Failed to query items', e);
+  props.onError?.(e, 'query');
   setItemList([]);
   setCurrentItem(null);
 }

Also applies to: 144-146, 298-304

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro